Skip to content

SPARK-1632. Remove unnecessary boxing in compares in ExternalAppendOnlyM... #559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
sryza wants to merge 2 commits into from
Closed

SPARK-1632. Remove unnecessary boxing in compares in ExternalAppendOnlyM... #559

sryza wants to merge 2 commits into from

Conversation

@sryza
Copy link
Contributor

@sryza sryza commented Apr 25, 2014

...ap

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

Merged build triggered.

srowen
srowen reviewed Apr 25, 2014
View reviewed changes
core/src/main/scala/org/apache/spark/util/collection/ExternalAppendOnlyMap.scala Outdated
Copy link
Member

@srowen srowen Apr 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth saving the hashCodes rather than recomputing? it might be non-trivial

Copy link
Contributor Author

@sryza sryza Apr 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

Merged build started.

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

Merged build triggered.

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

Merged build started.

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14493/

@andrewor14
Copy link
Contributor

andrewor14 commented Apr 25, 2014

LGTM

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

AmplabJenkins commented Apr 25, 2014

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14495/

@pwendell
Copy link
Contributor

pwendell commented Apr 26, 2014

Thanks, merged.

asfgit pushed a commit that referenced this pull request Apr 26, 2014
@sryza @pwendell
SPARK-1632. Remove unnecessary boxing in compares in ExternalAppendOn…
94c71e0 
…lyM...

...ap

Author: Sandy Ryza <[email protected]>

Closes #559 from sryza/sandy-spark-1632 and squashes the following commits:

a6cd352 [Sandy Ryza] Only compute hashes once
04e3884 [Sandy Ryza] SPARK-1632. Remove unnecessary boxing in compares in ExternalAppendOnlyMap
(cherry picked from commit 87cf35c)

Signed-off-by: Patrick Wendell <[email protected]>
@asfgit asfgit closed this in 87cf35c Apr 26, 2014
mridulm
mridulm reviewed Apr 27, 2014
View reviewed changes
core/src/main/scala/org/apache/spark/util/collection/ExternalAppendOnlyMap.scala
Copy link
Contributor

@mridulm mridulm Apr 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to use Integer.compare instead of writing these manually.

Copy link
Member

@srowen srowen Apr 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java 7 only :(
http://docs.oracle.com/javase/7/docs/api/java/lang/Integer.html#compare(int, int)

Guava has Ints.compare() though:
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/primitives/Ints.html#compare(int, int)

Copy link
Contributor

@andrewor14 andrewor14 Apr 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also write our own Utils.compare, though this is only used in 2 places so I think it's fine to just write it out directly. Also this is a super hot code path so maybe it's better to just avoid function calls altogether (maybe, maybe not).

pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
@sryza @pdeyhim
SPARK-1632. Remove unnecessary boxing in compares in ExternalAppendOn…
28286ab 
…lyM...

...ap

Author: Sandy Ryza <[email protected]>

Closes apache#559 from sryza/sandy-spark-1632 and squashes the following commits:

a6cd352 [Sandy Ryza] Only compute hashes once
04e3884 [Sandy Ryza] SPARK-1632. Remove unnecessary boxing in compares in ExternalAppendOnlyMap
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
@wangxiyuan @kiwik
Fix Cloud-Provider-OpenStack job error (apache#559)
4c02bb0 
1. The encrypt dockerhub user/password is wrong. It should not contain
\n.
2. The image build job pipline is changed to `periodic-10/22`.
3. Correct jinjia format.
arjunshroff pushed a commit to arjunshroff/spark that referenced this pull request Nov 24, 2020
@ekrivokonmapr
MapR [SPARK-598] Spark can't add needed properties to hive-site.xml (a…
42a1708 
…pache#559)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sryza @AmplabJenkins @andrewor14 @pwendell @srowen @mridulm